Skip to content

Conversation

@jonrohan
Copy link
Member

@jonrohan jonrohan commented Oct 5, 2017

Because of the ::before, ::after hack, the code blocks are not fully selectable. This pr removes the hack. It's been so long since this was introduced (~3 years ago) It's not fully clear what this was preventing. I think it has to do with scrollbars in Firefox and Android browsers.

I'd like to try and roll that back, then test on prod to see what bugs might crop up.

cc https://github.com/github/github/issues/58168

Copy link
Contributor

@broccolini broccolini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Down with testing this in github, don't want to approve until we've checked that doesn't break things though!

@@ -1,2 +1 @@
<link rel="stylesheet" href="https://unpkg.com/primer-css@9.2.0/build/build.css">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

padding: 0;
padding-top: 0.2em;
padding-bottom: 0.2em;
padding: 0.2em 0.4em;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One day in the future we should bring this into our proposed em-based scale and stick to common fractions.

letter-spacing: -0.2em; // this creates padding
content: "\00a0";
}
border-radius: 3px;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the $border-radius variable 😝

@@ -0,0 +1,18 @@
import React from 'react'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 Yay for adding more stories! It would be great to add the rest of the markdown styles when you get a chance.

@jonrohan
Copy link
Member Author

jonrohan commented Oct 6, 2017

This was deployed to review-lab https://github.com/github/github/pull/79702 and I didn't find any regressions.

@jonrohan jonrohan added the v10 label Oct 27, 2017
@broccolini broccolini mentioned this pull request Nov 6, 2017
28 tasks
@jonrohan jonrohan changed the base branch from dev to release-10.0.0 November 8, 2017 19:08
Copy link
Contributor

@broccolini broccolini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants